feat: add compliant MCP tool surface via MCP_COMPLIANCE_MODE [DO NOT MERGE]#166
Open
Xrazik1 wants to merge 1 commit into
Open
feat: add compliant MCP tool surface via MCP_COMPLIANCE_MODE [DO NOT MERGE]#166Xrazik1 wants to merge 1 commit into
Xrazik1 wants to merge 1 commit into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (34)
🚧 Files skipped from review as they are similar to previous changes (31)
WalkthroughAdds ChangesCompliance Mode Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant getConfig
participant registerSurface
participant CompliancePolicy
participant MCPTools
participant StatusResource
Environment->>getConfig: Read MCP_COMPLIANCE_MODE
getConfig->>registerSurface: Provide complianceMode
registerSurface->>CompliancePolicy: Evaluate isCompliant(config)
CompliancePolicy->>MCPTools: Select compliant or full registrations and schemas
MCPTools-->>registerSurface: Register selected surface
StatusResource-->>Environment: Report compliant or full surface
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
d2ba0a6 to
7880934
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 124-135: The compliance surface logging in src/index.ts only
distinguishes compliant, unset, and explicit opt-out, but it silently treats
invalid MCP_COMPLIANCE_MODE values as compliant; update the startup logic around
registerSurface and the complianceSurface computation to detect any non-accepted
normalized value and emit an explicit warning before logging the surface. Use
the existing parseComplianceMode/complianceMode flow to identify invalid inputs,
and keep the current console.error boot log while adding a clear warning path
for typoed or mis-scoped environment values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44dbf51f-90a0-4761-94fa-77fd1e06e826
📒 Files selected for processing (11)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.md
✅ Files skipped from review due to trivial changes (5)
- src/skills/dynamic-content.md
- src/skills/screenshots.md
- src/skills/cookie-consent.md
- src/skills/modals.md
- README.md
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
7880934 to
dd2239c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tools/compliance.ts (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHand-synced skill list risks drift.
COMPLIANT_SKILLS(Line 27) andCOMPLIANT_SKILL_TOOL_DESCRIPTION's bullet list (Line 82) are currently in sync but independently maintained. Adding/removing an id from the allowlist won't update the description text, and nothing will catch the mismatch.Consider generating the bullet list from
COMPLIANT_SKILLS(or a shared id→blurb map) so the two can't diverge.Also applies to: 77-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/compliance.ts` around lines 27 - 36, Eliminate the independently maintained skill bullet list by deriving COMPLIANT_SKILL_TOOL_DESCRIPTION from COMPLIANT_SKILLS, or from a shared skill-id-to-blurb mapping that also defines the allowlist. Ensure adding or removing a compliant skill automatically updates the description while preserving the existing ordering and wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/schemas.ts`:
- Around line 751-784: Update compliantCommandSchemas to remove the
action-capable ClickCommandSchema, TypeCommandSchema, SelectCommandSchema,
CheckboxCommandSchema, HoverCommandSchema, and UploadFileCommandSchema, keeping
only commands permitted by the navigation/read-only policy. Since
COMPLIANT_AGENT_METHODS derives from this list, ensure both schema validation
and the run-layer guard reject each removed method, and add rejection tests
covering all six methods.
---
Nitpick comments:
In `@src/tools/compliance.ts`:
- Around line 27-36: Eliminate the independently maintained skill bullet list by
deriving COMPLIANT_SKILL_TOOL_DESCRIPTION from COMPLIANT_SKILLS, or from a
shared skill-id-to-blurb mapping that also defines the allowlist. Ensure adding
or removing a compliant skill automatically updates the description while
preserving the existing ordering and wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0eab4659-a07c-43c3-9c93-8bf472d7c54f
📒 Files selected for processing (34)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.mdsrc/skills/snapshot-misses.mdsrc/tools/agent.tssrc/tools/compliance.tssrc/tools/export.tssrc/tools/performance.tssrc/tools/register.tssrc/tools/schemas.tssrc/tools/search.tstest/integration/server.spec.tstest/lib/api-client.spec.tstest/lib/config.spec.tstest/resources/resources.spec.tstest/skills/skills.spec.tstest/tools/agent.spec.tstest/tools/annotations.spec.tstest/tools/compliance-mode.spec.tstest/tools/crawl.spec.tstest/tools/export.spec.tstest/tools/function.spec.tstest/tools/map.spec.tstest/tools/performance.spec.tstest/tools/search.spec.tstest/tools/smartscraper.spec.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/skills/screenshots.md
- src/skills/cookie-consent.md
- src/skills/dynamic-content.md
- src/tools/export.ts
- src/resources/status.ts
- README.md
- src/skills/shadow-dom.md
- src/skills/modals.md
- src/index.ts
- src/skills/index.ts
- src/tools/agent.ts
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
dd2239c to
f5293aa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/tools/compliance-mode.spec.ts`:
- Around line 274-283: Update the compliance-mode test to define the eight
approved skill IDs as an independent expected allowlist rather than deriving
them from COMPLIANT_SKILLS. Use this fixed list to assert both the
COMPLIANT_SKILLS contents and schema acceptance, ensuring accidental changes to
COMPLIANT_SKILLS cannot make the assertions pass.
- Around line 439-450: Update the runtime allowlist test in
compliance-mode.spec.ts to invoke an unknown method instead of evaluate,
ensuring it verifies fail-closed behavior for non-allowlisted commands. Leave
the later batch-index test responsible for covering the known prohibited
evaluate method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e58d497-6929-4fce-acbc-0220dd45ecf6
📒 Files selected for processing (34)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.mdsrc/skills/snapshot-misses.mdsrc/tools/agent.tssrc/tools/compliance.tssrc/tools/export.tssrc/tools/performance.tssrc/tools/register.tssrc/tools/schemas.tssrc/tools/search.tstest/integration/server.spec.tstest/lib/api-client.spec.tstest/lib/config.spec.tstest/resources/resources.spec.tstest/skills/skills.spec.tstest/tools/agent.spec.tstest/tools/annotations.spec.tstest/tools/compliance-mode.spec.tstest/tools/crawl.spec.tstest/tools/export.spec.tstest/tools/function.spec.tstest/tools/map.spec.tstest/tools/performance.spec.tstest/tools/search.spec.tstest/tools/smartscraper.spec.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- src/skills/dynamic-content.md
- test/tools/crawl.spec.ts
- src/skills/screenshots.md
- test/tools/export.spec.ts
- src/@types/types.d.ts
- test/tools/annotations.spec.ts
- test/tools/search.spec.ts
- test/tools/map.spec.ts
- test/lib/api-client.spec.ts
- test/integration/server.spec.ts
- src/resources/status.ts
- test/tools/function.spec.ts
- src/tools/search.ts
- test/tools/smartscraper.spec.ts
- src/skills/cookie-consent.md
- src/skills/shadow-dom.md
- test/resources/resources.spec.ts
- src/skills/snapshot-misses.md
- src/config.ts
- src/tools/schemas.ts
- src/tools/export.ts
- src/skills/index.ts
- test/skills/skills.spec.ts
- src/skills/modals.md
- test/lib/config.spec.ts
- src/tools/performance.ts
- src/tools/register.ts
- src/tools/compliance.ts
- src/index.ts
- src/tools/agent.ts
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
f5293aa to
204e64f
Compare
andyMrtnzP
requested changes
Jul 13, 2026
| 'Fetches the URL and returns its content in the native format ' + | ||
| '(HTML, PDF, image, etc.). Automatically detects the content type.'; | ||
|
|
||
| export const COMPLIANT_SKILL_TOOL_DESCRIPTION = `Load a Browserless agent skill on demand. |
Contributor
There was a problem hiding this comment.
Hmm, these barebone rules pretty much kill the whole ReAct loop of the agent. We can still skeep a great part of the loop, stripping the parts that would go against their TOS
## Core Loop (ReAct: Reason → Act → Observe)
0. **Plan + check for a site recipe** — restate the goal, decide the target host, then \`browserless_skill { site: "<host>" }\` (see above). Load and follow any matching recipe before writing your own plan. Never jump straight to \`goto\`.
1. **goto** — waits "domcontentloaded"
2. **snapshot** — returns interactive + informational elements (button, link, textbox, combobox, checkbox, heading, img+alt) with ref= selectors
3. **Plan** all actions from snapshot
4. **Batch** execute
5. **Re-snapshot** only if page changed
6. Repeat → **close** when done
## Terminal-Goal Check
Before declaring done, restate the user's terminal deliverable in one line and verify your evidence *directly* supports it — not a sibling question.
## Snapshot Rules
- Until you snapshot a page, you CANNOT click/type/interact — snapshot first, no exceptions
- NEVER guess, assume, or infer selectors — CSS selectors from your training data are wrong. ONLY use ref= / deep-ref= from latest snapshot
- Snapshot STALE after: click, goto, select, navigation
- Snapshot VALID after: type, hover, scroll, evaluate
- Expect new content? → re-snapshot
- Element roles in snapshot (link, button, textbox, combobox, checkbox, heading) tell you what each does
- Snapshots after the first return a **diff** vs. your previous snapshot: only \`+\` new / \`~\` changed / \`-\` removed elements, plus a count of unchanged ones omitted. Unchanged elements stay valid — keep using their refs from the earlier snapshot. If that earlier snapshot is no longer in your context (summarized/trimmed away), request \`snapshot { full: true }\` to get the complete element list again.
## Selectors
- Use **ref=** (CSS) or **deep-ref=** (starts \`< \`) exactly as shown in snapshot
- Example: \`[3] button "Sign In" ref=button#submit\` → \`"button#submit"\`
- deep-ref for shadow DOM / iframes — see \`shadow-dom\` skill
## Content Extraction
1. Check in-memory snapshot (text/values already there)
2. **text** { selector } — from specific element
3. **evaluate** { content } — JS (IIFE): \`(() => { return ... })()\`
4. **html** { selector } — raw HTML
## Error Recovery
Errors tagged \`Category: <NAME>\`:
- **SELECTOR_MISS** — re-snapshot; retry \`< selector\` if not already deep-ref
- **SESSION_LOST** — a fresh session was opened automatically; re-goto + snapshot (prior state gone)
- **UNAUTHORIZED** / **FORBIDDEN** — pick different path
- **NOT_FOUND** — different URL
- **SERVER_ERROR** — backoff, retry once
- **NAVIGATION_FAILED** — verify URL
- **TIMEOUT** — longer wait or different signal
- **INVALID_PARAMS** — fix params (schema authoritative)
- **UNKNOWN** — re-snapshot + re-plan
Without mentioning auth, proxies or any way to impersonate an actual browser use, just smarter compliant scrape
| 'dynamic-content', | ||
| 'screenshots', | ||
| 'tabs', | ||
| 'file-transfers', |
Contributor
There was a problem hiding this comment.
I actually think this can be against their TOS... that's a way to bypass/impersonate a human uploading things. The vendors want us to just be a web bot
| 'Authentication profiles are not available on this endpoint.', | ||
| ); | ||
| } | ||
| if (params.proxy !== undefined) { |
Contributor
There was a problem hiding this comment.
Just to be extra sure that users can't prompt engineer their way into using proxies, we should also add a condition in the buildAgentWsUrl function
| // Required (not optional): the compliant surface is a security gate, so every | ||
| // McpConfig must choose explicitly — an omitted field must not default to the | ||
| // fuller/prohibited surface. | ||
| complianceMode: boolean; |
Contributor
There was a problem hiding this comment.
General question (not a necessary change). Since we are planning to eventually enable WebBot Auth compliance, would it be a good question to rename these references to webBotAuth mode? They are not strictly the same, but just a thought
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in compliant tool surface to the MCP server, selected per-process by the
MCP_COMPLIANCE_MODEenv var. When enabled, the server advertises a reduced, policy-compliant surface intended for the OpenAI / Anthropic app-directory listings (which reject the full surface for arbitrary code, scraping, and circumvention). The default full surface (flag off) is unchanged — verified byte-identical tomain.The compliant policy is centralized in
src/tools/compliance.ts; registration is gated insrc/tools/register.ts.What changes in compliant mode
smartscraper,function,map, andcrawl, plus the smartscraper-centricapi-docsresource and thescrape-url/extract-contentprompts. Registration is a data-driven surface table — every tool is classifiedboth/full, so a newly added tool is off the compliant surface by default.browserless_agentde-fanged at the schema level (strict — rejects, never silently strips): navigation/read commands only; nosolve/evaluate/loadSecret; no raw-BQL method passthrough; no top-levelproxy,profile, orcreateProfile.profilehydrates a saved auth session, so it is dropped as circumvention-adjacent.export/search/performanceuse.pick().strict()allowlists: dropscrapeOptions(search),includeResources(export), andprofile(all three). Run-layer guards back each schema.captchas,autonomous-login,auth-profile). The 8 allowed skill bodies are de-fanged at render — full-only passages (evaluate techniques, captcha selectors) are stripped via<!-- compliant-omit -->markers, with<!-- compliant-only -->replacements so each recipe stays coherent. Markers are validated at load and fail closed on anything malformed. Site-recipe pointers are suppressed in compliant agent replies (recipes can prescribe proxy/evaluate/login).surface: "compliant" | "full"; the boot log states the active surface.MCP_COMPLIANCE_MODEparsing is fail-closed — any set value other thanfalse/0/no/offenables the compliant surface.Testing
test/tools/compliance-mode.spec.tsguards the surface (exact tool set, schema accept/reject, run-layer defense, skill-body de-fang, marker validation, drift guards);test/lib/config.spec.tscovers fail-closed flag parsing. 515 passing, lint clean.Deploy
Full stays on
mcp.browserless.io/mcp; compliant serves at/mcp/connector(a second container withMCP_COMPLIANCE_MODE=true). Path routing is infra, not this repo — browserless/terraform#293 (connector container + LB) and browserless/load-balancer#94 (nginx route). This image is the prerequisite.[DO NOT MERGE] — PoC.
Summary by CodeRabbit
MCP_COMPLIANCE_MODEto select a reduced, directory-compliant tool surface, with unknown values handled fail-closed to compliant behavior and the warning surfaced at startup.complianceModein runtime configuration and centralized surface/tool registration so compliant vs full surfaces differ.compliantvsfull).MCP_COMPLIANCE_MODEin the environment variables section.